I think you can use dedup_by instead, assuming that ranges are in order. I'll check if they are.

I think you can use dedup_by instead, assuming that ranges are in order. I'll check if they are.

I couldn't find a way to use dedup_by, but @austinletson helped me with a way to do it in a more elegant way using a fold. Is this more along the lines of what you were thinking?

I think you can use dedup_by instead, assuming that ranges are in order. I'll check if they are.

I couldn't find a way to use dedup_by, but @austinletson helped me with a way to do it in a more elegant way using a fold. Is this more along the lines of what you were thinking?

The problem with this version is that you are potentially allocating a vector and copying all data. It might not be super critical but it is pretty easy to avoid here by using dedup_by which also is easier to read/clearer to understand (this use-case is exactly what that function is for). dedup_by iterates the vec in order and passes a pair of elements to your function. If your function returns true the first element is removed and only the second one is kept, otherwise both are kept. Furthermore you receive a mutable reference so you can modify both elements. I would use something along the following lines (it didn't test this and might have messed something up but it should point you in the right direction).

self.ranges.deadup_by(|prev_range, curr_range|{
      if prev_range.to() == curr_range.from()  {
          *curr_range = curr_range.merge(curr_range);
          // prev_range will be removed
          true
      } else {
          false
      }
})

The problem with this version is that you are potentially allocating a vector and copying all data. It might not be super critical but it is pretty easy to avoid here by using dedup_by which also is easier to read/clearer to understand (this use-case is exactly what that function is for). dedup_by iterates the vec in order and passes a pair of elements to your function. If your function returns true the first element is removed and only the second one is kept, otherwise both are kept. Furthermore you receive a mutable reference so you can modify both elements. I would use something along the following lines (it didn't test this and might have messed something up but it should point you in the right direction).

self.ranges.deadup_by(|prev_range, curr_range|{
      if prev_range.to() == curr_range.from()  {
          *curr_range = curr_range.merge(curr_range);
          // prev_range will be removed
          true
      } else {
          false
      }
})

Thanks for the help! I had missed the fact that dedup_by gives mutable refs; reimplemented the command.

Oh this is a nice implementation and we could simplify normalize in a similar way. The only difference is the conditional: if prev_range.to() == curr_range.from() vs if prev_range.overlaps(curr_range)

Oh this is a nice implementation and we could simplify normalize in a similar way. The only difference is the conditional: if prev_range.to() == curr_range.from() vs if prev_range.overlaps(curr_range)

Simplified normalize the same way, happy to move it to a different PR if that would be better.

Could you add a line for the new command to the book? Somewhere in this section would be good:

Added command to book, and renamed it to merge_consecutive_selections to fit better with the nomenclature used in other commands.